[SDK] feat: Add session key update handling and expose utility #6217

joaquim-verges · 2025-02-10T08:31:31Z

PR-Codex overview

This PR focuses on enhancing session key management in the thirdweb library by adding a new function, shouldUpdateSessionKey, which determines if a session key's permissions should be updated based on various conditions.

Detailed summary

Added shouldUpdateSessionKey function in packages/thirdweb/src/extensions/erc4337/account/addSessionKey.ts.
Updated connectSmartAccount function to utilize shouldUpdateSessionKey for session key checks.
Modified imports to include shouldUpdateSessionKey where necessary.
Added tests for shouldUpdateSessionKey in packages/thirdweb/src/extensions/erc4337/account/permissions.test.ts.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

vercel · 2025-02-10T08:31:35Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs-v2	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 10, 2025 7:45pm
login	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 10, 2025 7:45pm
thirdweb_playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 10, 2025 7:45pm
thirdweb-www	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 10, 2025 7:45pm
wallet-ui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 10, 2025 7:45pm

changeset-bot · 2025-02-10T08:31:35Z

🦋 Changeset detected

Latest commit: f42af42

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
thirdweb	Patch
@thirdweb-dev/wagmi-adapter	Patch
thirdweb-login	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

joaquim-verges · 2025-02-10T08:31:49Z

[SDK] feat: Add session key update handling and expose utility #6217 👈 (View in Graphite)
main

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

merge-queue - adds this PR to the back of the merge queue
hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

github-actions · 2025-02-10T08:35:22Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
thirdweb (esm)	46.28 KB (0%)	926 ms (0%)	690 ms (+151.63% 🔺)	1.7 s
thirdweb (cjs)	122.06 KB (0%)	2.5 s (0%)	881 ms (-12.91% 🔽)	3.4 s
thirdweb (minimal + tree-shaking)	5.6 KB (0%)	112 ms (0%)	88 ms (+214.72% 🔺)	200 ms
thirdweb/chains (tree-shaking)	506 B (0%)	10 ms (0%)	32 ms (+225.35% 🔺)	42 ms
thirdweb/react (minimal + tree-shaking)	19.3 KB (+0.03% 🔺)	387 ms (+0.03% 🔺)	197 ms (+37.45% 🔺)	583 ms

codecov · 2025-02-10T08:39:16Z

Codecov Report

Attention: Patch coverage is 96.92308% with 2 lines in your changes missing coverage. Please review.

Project coverage is 56.78%. Comparing base (c86e13b) to head (f42af42).
Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...eb/src/extensions/erc4337/account/addSessionKey.ts	96.49%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6217      +/-   ##
==========================================
+ Coverage   56.72%   56.78%   +0.05%     
==========================================
  Files        1161     1161              
  Lines       64289    64342      +53     
  Branches     5196     5219      +23     
==========================================
+ Hits        36468    36535      +67     
+ Misses      27095    27079      -16     
- Partials      726      728       +2

Flag	Coverage Δ		*Carryforward flag
legacy_packages	`65.68% <ø> (ø)`		Carriedforward from c86e13b
packages	`54.97% <96.92%> (+0.07%)`	⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines	Coverage Δ
packages/thirdweb/src/wallets/smart/index.ts	`61.03% <100.00%> (-0.34%)`	⬇️
...eb/src/extensions/erc4337/account/addSessionKey.ts	`95.06% <96.49%> (+3.39%)`	⬆️

... and 5 files with indirect coverage changes

alecananian · 2025-02-10T15:35:45Z

packages/thirdweb/src/extensions/erc4337/account/addSessionKey.ts

+  if (
+    !areSessionKeyContractTargetsEqual(
+      currentPermissions.approvedTargets,
+      newPermissions.approvedTargets,
+    )


Instead of checking for equality, it could check for subests to minimize the need for creating sessions (i.e., currentPermissions.approvedTargets includes at least all of newPermissions.approvedTargets)

Thank you! Looks great

alecananian · 2025-02-10T15:36:13Z

packages/thirdweb/src/extensions/erc4337/account/addSessionKey.ts

+  if (
+    currentPermissions.nativeTokenLimitPerTransaction !==
+    toWei(newPermissions.nativeTokenLimitPerTransaction?.toString() ?? "0")


Again instead of equality, this could be currentPermissions.nativeTokenLimitPerTransaction >= newPermissions.nativeTokenLimitPerTransaction

alecananian · 2025-02-10T15:38:18Z

packages/thirdweb/src/extensions/erc4337/account/addSessionKey.ts

+ * @param newPermissions - The new permissions to set for the session key.
+ * @returns A boolean indicating if the session key should be updated.
+ */
+export async function shouldUpdateSessionKey(args: {


We also check on startDate and isAdmin values in our current version of this logic, but not sure if that's important here.

start date is a bit weird, decided to leave it alone for this. And yeah admin should be separate i think?

github-actions bot assigned joaquim-verges Feb 10, 2025

github-actions bot added packages SDK Involves changes to the thirdweb SDK labels Feb 10, 2025

joaquim-verges marked this pull request as ready for review February 10, 2025 08:31

joaquim-verges requested review from a team as code owners February 10, 2025 08:31

joaquim-verges mentioned this pull request Feb 10, 2025

Smart wallet session key auto-creation doesn't take into account permissions when skipping #6203

Closed

vercel bot deployed to Preview – wallet-ui February 10, 2025 08:34 View deployment

vercel bot deployed to Preview – thirdweb_playground February 10, 2025 08:34 View deployment

vercel bot deployed to Preview – login February 10, 2025 08:36 View deployment

vercel bot had a problem deploying to Preview – docs-v2 February 10, 2025 08:36 Failure

vercel bot deployed to Preview – thirdweb-www February 10, 2025 08:38 View deployment

alecananian reviewed Feb 10, 2025

View reviewed changes

[SDK] feat: Add session key update handling and expose utility

f42af42

joaquim-verges force-pushed the _SDK_feat_Add_session_key_update_handling_and_expose_utility branch from 004598e to f42af42 Compare February 10, 2025 19:35

vercel bot deployed to Preview – login February 10, 2025 19:38 View deployment

vercel bot deployed to Preview – wallet-ui February 10, 2025 19:38 View deployment

vercel bot deployed to Preview – thirdweb_playground February 10, 2025 19:38 View deployment

vercel bot deployed to Preview – docs-v2 February 10, 2025 19:43 View deployment

vercel bot deployed to Preview – thirdweb-www February 10, 2025 19:45 View deployment

joaquim-verges merged commit 6b2a90a into main Feb 11, 2025
33 of 34 checks passed

joaquim-verges deleted the _SDK_feat_Add_session_key_update_handling_and_expose_utility branch February 11, 2025 03:40

joaquim-verges mentioned this pull request Feb 11, 2025

Version Packages #6208

Merged

[SDK] feat: Add session key update handling and expose utility #6217

[SDK] feat: Add session key update handling and expose utility #6217

Conversation

joaquim-verges commented Feb 10, 2025 • edited by pr-codex bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR-Codex overview

Detailed summary

Uh oh!

vercel bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

joaquim-verges commented Feb 10, 2025

How to use the Graphite Merge Queue

Uh oh!

github-actions bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

codecov bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alecananian Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

joaquim-verges Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

alecananian Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

alecananian Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

joaquim-verges Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

alecananian Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

joaquim-verges Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joaquim-verges commented Feb 10, 2025 •

edited by pr-codex bot

Loading

vercel bot commented Feb 10, 2025 •

edited

Loading

changeset-bot bot commented Feb 10, 2025 •

edited

Loading

github-actions bot commented Feb 10, 2025 •

edited

Loading

codecov bot commented Feb 10, 2025 •

edited

Loading